Skip to content

Modified Arduino.h with changes to the boolean type #2151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Modified Arduino.h with changes to the boolean type #2151

wants to merge 3 commits into from

Conversation

Chris--A
Copy link
Contributor

As the Arduino specific boolean type is dangerous in C++, I have implemented a fix to solve the situation for both C and C++ There is also no need for stdbool, it also defines true and false, which is not needed or allowed in a C++ context.

This is in reference to the issue I posted here.

#2147

As the Arduino specific boolean type is dangerous in C++, I have implemented a fix to solve the situation for both C and C++ There is also no need for stdbool, it also defines true and false, which is not needed or allowed in a C++ context.
@Chris--A
Copy link
Contributor Author

I have modified the code to only have the changes. I also replied here #2147 (comment) as to why my idea is worth implementing. For the features stdbool provides, I recommend adding it to only add in C if it must be used, there is no reason to change the current bool, true, or false in C++. And simply to avoid compilation overhead it can be left out altogether, Arduino.h does get included extensively. Also you pointed out that _Bool is an extension for gcc, why not just use the standard implementation (when in C++).

@Chris--A
Copy link
Contributor Author

Chris--A commented Jul 2, 2014

Also, if you'd rather keep the stdbool functionality, I can modify my pull request to leave it in.
My main goal here is to get the boolean data type fixed for C++ code. The true/false problem is solved using stdbool or my original code so that is not an issue anymore.

@@ -32,6 +31,14 @@
#include "binary.h"

#ifdef __cplusplus
typedef bool boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the important part of my addition. The boolean type needs to be seen as a bool when compiling C++ code.

This update will allow the String class to be used in a range based for loop.
@ArduinoBot
Copy link
Contributor

Can one of the admins verify this patch?

@xxxajk
Copy link
Contributor

xxxajk commented Dec 16, 2014

I think that stdbool.h should be used, as it will actually do the right thing.
Next, use a define for boolean and define it as _Bool instead of using a typedef.

@Chris--A
Copy link
Contributor Author

Stdbool.h doesn't really solve anything though. As you can see its the typedef in Arduino.h which is the problem. bool is just an alias for _Bool in C code. Infact in C++ code it is defined as: #define _Bool bool, which leaves no need for the library, or the time an additional include imposes on the compilation. (Arduino.h with my small changes already implements what stdbool.h does in its entirety ).

@xxxajk
Copy link
Contributor

xxxajk commented Dec 19, 2014

Yes, but is the type correct? I can't tell from the diff.

@Chris--A
Copy link
Contributor Author

It effectively adds this:

 #ifdef __cplusplus
    typedef bool boolean;
#else
    typedef uint8_t boolean;
    #define false 0
    #define true !false
#endif

So in C++ boolean is an alias for bool.
And in C boolean is an alias for an unsigned char.
The standard only explicitly gives false a value, so true is its opposite (!false), and in C++ they are actual bool types provided by the standard.

Basically I do not mind how this is implemented, I just insist that the Arduino custom type be a real bool, and true/false not be defines. (in C++). This is a serious bug in my opinion.

Also stdbool.h uses a GCC specific _Bool, so if someone changes the platform.txt file to use -c++98/-c++11 instead of -gnu++98/-gnu++11 the Arduino API will not compile.

The third commit doesn't belong here, If this fix is going to be used I will remove it and merge the remaining commits once I can get Arduino cloned on my PC.

@xxxajk
Copy link
Contributor

xxxajk commented Dec 21, 2014

I wouldn't worry so much with porting. I highly doubt the Arduino team will
use anything other than GNU tools in the foreseeable future.

On Sun, Dec 21, 2014 at 7:33 AM, Christopher Andrews <
notifications@github.com> wrote:

It effectively adds this:

#ifdef __cplusplus
typedef bool boolean;
#else
typedef uint8_t boolean;
#define false 0
#define true !false
#endif

So in C++ boolean is an alias for bool.
And in C boolean is an alias for an unsigned char.
The standard only explicitly gives false a value, so true is its opposite
(!false), and in C++ they are actual bool types provided by the standard.

Basically I do not mind how this is implemented, I just insist that the
Arduino custom type be a real bool, and true/false not be defines. (in
C++). This is a serious bug in my opinion.

Also stdbool.h uses a GCC specific _Bool, so if someone changes the
platform.txt file to use -c++98/-c++11 instead of -gnu++98/-gnu++11 the
Arduino API will not compile.

The third commit doesn't belong here, If this fix is going to be used I
will remove it and merge the remaining commits once I can get Arduino
cloned on my PC.


Reply to this email directly or view it on GitHub
#2151 (comment).

Visit my github for awesome Arduino code @ https://github.com/xxxajk

PaulStoffregen added a commit to PaulStoffregen/cores that referenced this pull request Dec 21, 2014
@cmaglie
Copy link
Member

cmaglie commented Jan 6, 2015

From:
http://www.cplusplus.com/reference/cstdbool/
http://www.c-faq.com/bool/booltype.html
http://stackoverflow.com/questions/1608318/is-bool-a-native-c-type

it seems that _Bool is not gcc specific but part of C99

The purpose in C of this header (stdbool.h) is to add a bool type and the true and false values as macro definitions.
In C++, which supports those directly, the header simply contains a macro that can be used to check if the type is supported:

So it's sufficient to include stdbool.h to have a proper bool type defined.
I'd like to leave all the boring definition of true/false etc. to stdbool.h and change only the following:

 #define bit(b) (1UL << (b))

-typedef uint8_t boolean;
+typedef bool boolean;
 typedef uint8_t byte;

 void init(void);

What do you think?

@xxxajk
Copy link
Contributor

xxxajk commented Jan 6, 2015

That is exactly what I was saying :-)

On Tue, Jan 6, 2015 at 4:43 PM, Cristian Maglie notifications@github.com
wrote:

From:
http://www.cplusplus.com/reference/cstdbool/
http://www.c-faq.com/bool/booltype.html
http://stackoverflow.com/questions/1608318/is-bool-a-native-c-type

it seems that _Bool is not gcc specific but part of C99

The purpose in C of this header (stdbool.h) is to add a bool type and the
true and false values as macro definitions.
In C++, which supports those directly, the header simply contains a macro
that can be used to check if the type is supported:

So it's sufficient to include stdbool.h to have a proper bool type
defined.
I'd like to leave all the boring definition of true/false etc. to
stdbool.h and change only the following:

#define bit(b) (1UL << (b))
-typedef uint8_t boolean;+typedef bool boolean;
typedef uint8_t byte;

void init(void);

What do you think?


Reply to this email directly or view it on GitHub
#2151 (comment).

Visit my github for awesome Arduino code @ https://github.com/xxxajk

@matthijskooijman
Copy link
Collaborator

+1

@PaulStoffregen
Copy link
Contributor

FWIW, I merged a version for this fix on Teensy, using this:

+// fix C++ boolean issue
+// https://github.com/arduino/Arduino/pull/2151
+#ifdef __cplusplus
+typedef bool boolean;
+#else
+typedef uint8_t boolean;
+#define false 0
+#define true (!false)
+#endif

@Chris--A
Copy link
Contributor Author

Chris--A commented Jan 7, 2015

@cmaglie That is excellent. All that is important to me is correct semantics in C++.

Sorry @xxajk, the GCC documentation led me to believe it was an extension/GCC only.

cmaglie added a commit that referenced this pull request Jan 7, 2015
@cmaglie
Copy link
Member

cmaglie commented Jan 7, 2015

Fixed with 20ac20f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants